Skip to content

Handle duplicate X-WR-TIMEZONE values - #43

Merged
niccokunzmann merged 3 commits into
pycalendar:mainfrom
legalverb:fix-duplicate-x-wr-timezone
Jun 11, 2026
Merged

Handle duplicate X-WR-TIMEZONE values#43
niccokunzmann merged 3 commits into
pycalendar:mainfrom
legalverb:fix-duplicate-x-wr-timezone

Conversation

@legalverb

Copy link
Copy Markdown
Contributor

Fixes #25.

Summary

  • Normalize duplicate calendar-provided X-WR-TIMEZONE values before resolving them with zoneinfo.
  • Use the duplicate value when all X-WR-TIMEZONE entries agree.
  • Treat conflicting duplicate values as ambiguous and leave the calendar unchanged instead of raising an attribute/type error.
  • Add regression coverage for matching and conflicting duplicate timezone metadata.

Behavior

The issue did not include the original ICS file, so this keeps the behavior conservative:

  • repeated identical calendar-level time zones are safe to use;
  • conflicting calendar-level time zones are ignored because the correct conversion target is ambiguous.

Tests

  • . .venv/bin/activate && pytest test/test_convert_examples.py -q
  • . .venv/bin/activate && pytest -q
  • sh test/test_code_quality.sh
  • git diff --check

Note: CONTRIBUTING.md asks for a changelog entry except for trivial changes, but I could not find a changelog/news file in this repository to update.

@niccokunzmann

Copy link
Copy Markdown
Member

Thank you for your PR!
I think, the changelog is in the README.

It would be nice if you could also edit the contributing file.

Comment thread test/test_convert_examples.py Outdated
DTSTART:20240101T120000Z
DTEND:20240101T130000Z
END:VEVENT
END:VCALENDAR

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, the tests have a calendars directory. Please put all calendars in there.

@legalverb
legalverb force-pushed the fix-duplicate-x-wr-timezone branch from 443cfba to 851e705 Compare June 5, 2026 02:23
@legalverb

Copy link
Copy Markdown
Contributor Author

Thanks for the review. I moved the duplicate X-WR-TIMEZONE test calendars into test/calendars and rebased the branch on current main, so the PR is mergeable again.

Local verification:

  • git diff --check: passed
  • Tried running the focused pytest targets, but this local venv is currently hanging while importing pytest/icalendar before test output. The change itself now uses the existing calendars fixture and fixture directory as requested.

@niccokunzmann niccokunzmann left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Perfect! Merging....

@niccokunzmann
niccokunzmann merged commit e963e94 into pycalendar:main Jun 11, 2026
1 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Calendar with more than one X-WR-TIMEZONE fails

2 participants